Build only lib from package #839

vhbit · 2014-11-11T13:38:55Z

Fixes #724

vhbit · 2014-11-11T13:39:15Z

alexcrichton · 2014-11-11T18:18:39Z

I think the plan was to add --name to the list of arguments to cargo build to stay in line with the rest of the cargo commands. I'd also be fine adding logic that just doesn't built executables for platforms that don't support executables instead of adding another flag.

vhbit · 2014-11-12T06:53:19Z

@alexcrichton hmm, my motivation was:

so far there could be only one library in the package so it is natural to skip its name as it is inferred
providing a name for build actually should also take into account that build is general and might be used for all kind of binaries (cargo build --bins #843 is about it).

So if sticking with the --name plan perhaps a better alternative is to provide type of build like: cargo build --name foo:lib or cargo build --name foo:bin --name foo:test --name foo:example. In this case --name could be skipped at all. But I'm not sure this actually is that much needed as plain lib.

I'd also be fine adding logic that just doesn't built executables for platforms that don't support executables instead of adding another flag.

It is not just about platform support - it also might speed up compilation when all you need is the library itself.

alexcrichton · 2014-11-13T06:01:05Z

Ok thinking about this, I think this may be the best way to go. The reason I didn't like --test foo for cargo test was that you stuttered with cargo test --test foo and there is only one type of target to run there (test targets!), so specifying --test seemed redundant.

Here though I don't think it's worth it to implement --name due to clashes. Requiring foo:lib means that it's somewhat counterintuitive, and implementing a system which accepts cargo build --name foo and succeeds with one target named foo and fails with more than one target named foo also seems overly complicated.

In the end I think that --lib and --bins are kinda the most expected of the bunch. This does, however lend to a bit of a clash between cargo build and the other commands which all accept --name across the board. I'm somewhat uneasy with that, but I could also live with it in the end.

I'll consult with a few others about this tomorrow and get back to you about what we think the best way to go about this is. Also cc @stepancheg, this is basically the same outcome for #843 I believe.

vhbit · 2014-11-13T17:07:43Z

hmm, any ideas why it fails cross compile tests on OS X and if it is really related to this patch (it seems to be an error with lib paths as it can't find std crate)?

alexcrichton · 2014-11-13T17:10:58Z

Ah it's ok, it looks like the download of the nightly failed which was probably spurious

alexcrichton · 2014-11-13T17:31:43Z

@vhbit, @stepancheg, alright, let's move forward with these PRs. I'll rename --name to --target-name for the other commands soon.

alexcrichton · 2014-11-13T17:32:24Z

tests/test_cargo_build_lib.rs

+
+    assert_that(p.cargo_process("build").arg("--lib"),
+                execs().with_status(0));
+    assert_that(&p.bin("foo"), is_not(existing_file()));


Can you test against the output with -v to ensure that the library actually was built?

Ok, isn't it enough just to test if lib file exists?

Yeah that'd also work.

With rust-lang#843 and rust-lang#839 coming around the bend soon, the original decision for `--name` everywhere isn't making as much sense, for consistence this is renaming these flags back to `--<target-name>` for the respective targets.

vhbit · 2014-11-14T07:59:52Z

Added error if no libs are presented, added a test case for it, also while building just lib checking verbose output.

alexcrichton · 2014-11-14T17:02:30Z

tests/tests.rs

@@ -27,6 +27,7 @@ macro_rules! test(



Looks like this file turned into an executable, could you flip it back?

alexcrichton · 2014-11-14T17:02:42Z

Looks great, thanks @vhbit!

Fixes rust-lang#724

vhbit · 2014-11-14T21:11:12Z

Ahh, that strange problem with emacs sometimes changing permissions only for .rs... Can't find the reason.

Anyway, fixed.

Fixes #724

With #843 and #839 coming around the bend soon, the original decision for `--name` everywhere isn't making as much sense, for consistence this is renaming these flags back to `--<target-name>` for the respective targets.

ryoqun · 2024-06-28T04:28:37Z

src/cargo/ops/cargo_compile.rs

+    }).filter(|target| !lib_only || target.is_lib()).collect::<Vec<&Target>>();
+
+    if lib_only && targets.len() == 0 {
+        return Err(human("There is no lib to build, remove `--lib` flag".to_string()));


this (back ref: https://github.com/rust-lang/cargo/pull/14163/files#r1658125218)

stepancheg mentioned this pull request Nov 11, 2014

cargo build --bins #843

Closed

vhbit force-pushed the lib-only branch from c14e028 to 3a2d5cf Compare November 13, 2014 16:29

alexcrichton reviewed Nov 13, 2014
View reviewed changes

alexcrichton mentioned this pull request Nov 13, 2014

Rename --name flags to --<target-name> #865

Merged

vhbit force-pushed the lib-only branch from 3a2d5cf to da08120 Compare November 14, 2014 07:58

alexcrichton reviewed Nov 14, 2014
View reviewed changes

tests/tests.rs

@@ -27,6 +27,7 @@ macro_rules! test(

Copy link

Member

alexcrichton Nov 14, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this file turned into an executable, could you flip it back?

Build only lib from package

9c1c363

Fixes rust-lang#724

vhbit force-pushed the lib-only branch from da08120 to 9c1c363 Compare November 14, 2014 21:10

bors added a commit that referenced this pull request Nov 14, 2014

auto merge of #839 : vhbit/cargo/lib-only, r=alexcrichton

bfcc65c

Fixes #724

bors merged commit 9c1c363 into rust-lang:master Nov 14, 2014

ryoqun reviewed Jun 28, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Build only lib from package #839

Build only lib from package #839

vhbit commented Nov 11, 2014

vhbit commented Nov 11, 2014

alexcrichton commented Nov 11, 2014

vhbit commented Nov 12, 2014

alexcrichton commented Nov 13, 2014

vhbit commented Nov 13, 2014

alexcrichton commented Nov 13, 2014

alexcrichton commented Nov 13, 2014

alexcrichton Nov 13, 2014

vhbit Nov 13, 2014

alexcrichton Nov 13, 2014

vhbit commented Nov 14, 2014

alexcrichton Nov 14, 2014

alexcrichton commented Nov 14, 2014

vhbit commented Nov 14, 2014

ryoqun Jun 28, 2024 •

edited

Loading

Build only lib from package #839

Build only lib from package #839

Conversation

vhbit commented Nov 11, 2014

vhbit commented Nov 11, 2014

alexcrichton commented Nov 11, 2014

vhbit commented Nov 12, 2014

alexcrichton commented Nov 13, 2014

vhbit commented Nov 13, 2014

alexcrichton commented Nov 13, 2014

alexcrichton commented Nov 13, 2014

alexcrichton Nov 13, 2014

Choose a reason for hiding this comment

vhbit Nov 13, 2014

Choose a reason for hiding this comment

alexcrichton Nov 13, 2014

Choose a reason for hiding this comment

vhbit commented Nov 14, 2014

alexcrichton Nov 14, 2014

Choose a reason for hiding this comment

alexcrichton commented Nov 14, 2014

vhbit commented Nov 14, 2014

ryoqun Jun 28, 2024 • edited Loading

Choose a reason for hiding this comment

ryoqun Jun 28, 2024 •

edited

Loading